From c6a25536693aeec176e7b55a533f5e1465f73e88 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Tue, 28 Nov 2006 11:43:39 +0000 Subject: [PATCH] [HVM] Fix MSR access code. - rdmsr/wrmsr always use ECX (not RCX) as register index. - SVM still had the function names explicitly in the HVM_DBG_LOG() output - the guest should (at the very minimum) see GP fault for MSRs accesses to which even fault in Xen itself Signed-off-by: Jan Beulich --- xen/arch/x86/hvm/svm/svm.c | 51 ++++++++++++++----------------- xen/arch/x86/hvm/vmx/vmx.c | 62 +++++++++++++++++++------------------- 2 files changed, 54 insertions(+), 59 deletions(-) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index cfcd4293c2..fa241f6c2d 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -277,7 +277,7 @@ static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) struct vcpu *vc = current; struct vmcb_struct *vmcb = vc->arch.hvm_svm.vmcb; - switch (regs->ecx) + switch ((u32)regs->ecx) { case MSR_EFER: msr_content = vmcb->efer; @@ -315,7 +315,7 @@ static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) return 0; } - HVM_DBG_LOG(DBG_LEVEL_2, "mode_do_msr_read: msr_content: %"PRIx64"\n", + HVM_DBG_LOG(DBG_LEVEL_2, "msr_content: %"PRIx64"\n", msr_content); regs->eax = (u32)(msr_content >> 0); @@ -329,11 +329,10 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) struct vcpu *v = current; struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; - HVM_DBG_LOG(DBG_LEVEL_1, "mode_do_msr_write msr %lx " - "msr_content %"PRIx64"\n", - (unsigned long)regs->ecx, msr_content); + HVM_DBG_LOG(DBG_LEVEL_1, "msr %x msr_content %"PRIx64"\n", + (u32)regs->ecx, msr_content); - switch ( regs->ecx ) + switch ( (u32)regs->ecx ) { case MSR_EFER: #ifdef __x86_64__ @@ -1855,22 +1854,18 @@ static inline void svm_do_msr_access( struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; int inst_len; u64 msr_content=0; - u32 eax, edx; + u32 ecx = regs->ecx, eax, edx; ASSERT(vmcb); - HVM_DBG_LOG(DBG_LEVEL_1, "svm_do_msr_access: ecx=%lx, eax=%lx, edx=%lx, " - "exitinfo = %lx", (unsigned long)regs->ecx, - (unsigned long)regs->eax, (unsigned long)regs->edx, + HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%x, eax=%x, edx=%x, exitinfo = %lx", + ecx, (u32)regs->eax, (u32)regs->edx, (unsigned long)vmcb->exitinfo1); /* is it a read? */ if (vmcb->exitinfo1 == 0) { - inst_len = __get_instruction_length(vmcb, INSTR_RDMSR, NULL); - - regs->edx = 0; - switch (regs->ecx) { + switch (ecx) { case MSR_IA32_TIME_STAMP_COUNTER: msr_content = hvm_get_guest_time(v); break; @@ -1890,25 +1885,30 @@ static inline void svm_do_msr_access( if (long_mode_do_msr_read(regs)) goto done; - if ( rdmsr_hypervisor_regs(regs->ecx, &eax, &edx) ) + if ( rdmsr_hypervisor_regs(ecx, &eax, &edx) || + rdmsr_safe(ecx, eax, edx) == 0 ) { regs->eax = eax; regs->edx = edx; goto done; } - - rdmsr_safe(regs->ecx, regs->eax, regs->edx); - break; + svm_inject_exception(v, TRAP_gp_fault, 1, 0); + return; } regs->eax = msr_content & 0xFFFFFFFF; regs->edx = msr_content >> 32; + + done: + HVM_DBG_LOG(DBG_LEVEL_1, "returns: ecx=%x, eax=%lx, edx=%lx", + ecx, (unsigned long)regs->eax, (unsigned long)regs->edx); + + inst_len = __get_instruction_length(vmcb, INSTR_RDMSR, NULL); } else { - inst_len = __get_instruction_length(vmcb, INSTR_WRMSR, NULL); msr_content = (u32)regs->eax | ((u64)regs->edx << 32); - switch (regs->ecx) + switch (ecx) { case MSR_IA32_TIME_STAMP_COUNTER: hvm_set_guest_time(v, msr_content); @@ -1927,17 +1927,12 @@ static inline void svm_do_msr_access( break; default: if ( !long_mode_do_msr_write(regs) ) - wrmsr_hypervisor_regs(regs->ecx, regs->eax, regs->edx); + wrmsr_hypervisor_regs(ecx, regs->eax, regs->edx); break; } - } - done: - - HVM_DBG_LOG(DBG_LEVEL_1, "svm_do_msr_access returns: " - "ecx=%lx, eax=%lx, edx=%lx", - (unsigned long)regs->ecx, (unsigned long)regs->eax, - (unsigned long)regs->edx); + inst_len = __get_instruction_length(vmcb, INSTR_WRMSR, NULL); + } __update_guest_eip(vmcb, inst_len); } diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 15614608fc..1d247b7d34 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -116,7 +116,7 @@ static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) struct vcpu *v = current; struct vmx_msr_state *guest_msr_state = &v->arch.hvm_vmx.msr_state; - switch ( regs->ecx ) { + switch ( (u32)regs->ecx ) { case MSR_EFER: HVM_DBG_LOG(DBG_LEVEL_2, "EFER msr_content 0x%"PRIx64, msr_content); msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_EFER]; @@ -169,10 +169,10 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) struct vmx_msr_state *guest_msr_state = &v->arch.hvm_vmx.msr_state; struct vmx_msr_state *host_msr_state = &this_cpu(host_msr_state); - HVM_DBG_LOG(DBG_LEVEL_1, "msr 0x%lx msr_content 0x%"PRIx64"\n", - (unsigned long)regs->ecx, msr_content); + HVM_DBG_LOG(DBG_LEVEL_1, "msr 0x%x msr_content 0x%"PRIx64"\n", + (u32)regs->ecx, msr_content); - switch ( regs->ecx ) { + switch ( (u32)regs->ecx ) { case MSR_EFER: /* offending reserved bit will cause #GP */ if ( msr_content & ~(EFER_LME | EFER_LMA | EFER_NX | EFER_SCE) ) @@ -1790,16 +1790,16 @@ static int vmx_cr_access(unsigned long exit_qualification, return 1; } -static inline void vmx_do_msr_read(struct cpu_user_regs *regs) +static inline int vmx_do_msr_read(struct cpu_user_regs *regs) { u64 msr_content = 0; - u32 eax, edx; + u32 ecx = regs->ecx, eax, edx; struct vcpu *v = current; - HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%lx, eax=%lx, edx=%lx", - (unsigned long)regs->ecx, (unsigned long)regs->eax, - (unsigned long)regs->edx); - switch (regs->ecx) { + HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%x, eax=%x, edx=%x", + ecx, (u32)regs->eax, (u32)regs->edx); + + switch (ecx) { case MSR_IA32_TIME_STAMP_COUNTER: msr_content = hvm_get_guest_time(v); break; @@ -1817,39 +1817,41 @@ static inline void vmx_do_msr_read(struct cpu_user_regs *regs) break; default: if ( long_mode_do_msr_read(regs) ) - return; + goto done; - if ( rdmsr_hypervisor_regs(regs->ecx, &eax, &edx) ) + if ( rdmsr_hypervisor_regs(ecx, &eax, &edx) || + rdmsr_safe(ecx, eax, edx) == 0 ) { regs->eax = eax; regs->edx = edx; - return; + goto done; } - - rdmsr_safe(regs->ecx, regs->eax, regs->edx); - return; + vmx_inject_hw_exception(v, TRAP_gp_fault, 0); + return 0; } regs->eax = msr_content & 0xFFFFFFFF; regs->edx = msr_content >> 32; - HVM_DBG_LOG(DBG_LEVEL_1, "returns: ecx=%lx, eax=%lx, edx=%lx", - (unsigned long)regs->ecx, (unsigned long)regs->eax, +done: + HVM_DBG_LOG(DBG_LEVEL_1, "returns: ecx=%x, eax=%lx, edx=%lx", + ecx, (unsigned long)regs->eax, (unsigned long)regs->edx); + return 1; } -static inline void vmx_do_msr_write(struct cpu_user_regs *regs) +static inline int vmx_do_msr_write(struct cpu_user_regs *regs) { + u32 ecx = regs->ecx; u64 msr_content; struct vcpu *v = current; - HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%lx, eax=%lx, edx=%lx", - (unsigned long)regs->ecx, (unsigned long)regs->eax, - (unsigned long)regs->edx); + HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%x, eax=%x, edx=%x", + ecx, (u32)regs->eax, (u32)regs->edx); msr_content = (u32)regs->eax | ((u64)regs->edx << 32); - switch (regs->ecx) { + switch (ecx) { case MSR_IA32_TIME_STAMP_COUNTER: { struct periodic_time *pt = @@ -1874,13 +1876,11 @@ static inline void vmx_do_msr_write(struct cpu_user_regs *regs) break; default: if ( !long_mode_do_msr_write(regs) ) - wrmsr_hypervisor_regs(regs->ecx, regs->eax, regs->edx); + wrmsr_hypervisor_regs(ecx, regs->eax, regs->edx); break; } - HVM_DBG_LOG(DBG_LEVEL_1, "returns: ecx=%lx, eax=%lx, edx=%lx", - (unsigned long)regs->ecx, (unsigned long)regs->eax, - (unsigned long)regs->edx); + return 1; } static void vmx_do_hlt(void) @@ -2244,16 +2244,16 @@ asmlinkage void vmx_vmexit_handler(struct cpu_user_regs *regs) break; case EXIT_REASON_MSR_READ: inst_len = __get_instruction_length(); /* Safe: RDMSR */ - __update_guest_eip(inst_len); - vmx_do_msr_read(regs); + if ( vmx_do_msr_read(regs) ) + __update_guest_eip(inst_len); TRACE_VMEXIT(1, regs->ecx); TRACE_VMEXIT(2, regs->eax); TRACE_VMEXIT(3, regs->edx); break; case EXIT_REASON_MSR_WRITE: inst_len = __get_instruction_length(); /* Safe: WRMSR */ - __update_guest_eip(inst_len); - vmx_do_msr_write(regs); + if ( vmx_do_msr_write(regs) ) + __update_guest_eip(inst_len); TRACE_VMEXIT(1, regs->ecx); TRACE_VMEXIT(2, regs->eax); TRACE_VMEXIT(3, regs->edx); -- 2.30.2